fix: pass Hub-hydrated harness-config path to harness.Resolve#450
Conversation
In broker mode, harness.Resolve() used FindHarnessConfigDir which only
searches local disk. Hub-managed harness configs hydrated into temp
directories were invisible to the resolver, causing it to fall back to
a Generic{} harness with no command config. This produced an empty
shell command (sh -c "") and the agent exited immediately.
Add ConfigDirPath to ResolveOptions so the broker can pass the
Hub-hydrated path directly. This ensures the harness config (including
no_auth, command, provisioner) is loaded correctly in broker mode.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for an explicit harness configuration directory path (ConfigDirPath) in ResolveOptions, allowing the system to load the harness configuration directly from a specified path instead of relying solely on name-based lookups. The reviewer recommended that if an explicit ConfigDirPath is provided but fails to load, the error should be treated as a hard error and returned immediately rather than being silently ignored, to prevent unexpected behavior and ease debugging.
| var hcDir *config.HarnessConfigDir | ||
| var hcErr error | ||
| if opts.ConfigDirPath != "" { | ||
| hcDir, hcErr = config.LoadHarnessConfigDir(opts.ConfigDirPath) | ||
| } else { | ||
| hcDir, hcErr = config.FindHarnessConfigDir(opts.Name, opts.ProjectPath, opts.TemplatePaths...) | ||
| } |
There was a problem hiding this comment.
If an explicit ConfigDirPath is provided but fails to load (e.g., due to a missing or malformed config.yaml), silently ignoring the error and falling back to a generic or built-in harness can lead to unexpected behavior and make debugging difficult. Since the path is explicitly provided, any failure to load it should be treated as a hard error.
var hcDir *config.HarnessConfigDir
var hcErr error
if opts.ConfigDirPath != "" {
hcDir, hcErr = config.LoadHarnessConfigDir(opts.ConfigDirPath)
if hcErr != nil {
return nil, fmt.Errorf("failed to load explicit harness config directory %q: %w", opts.ConfigDirPath, hcErr)
}
} else {
hcDir, hcErr = config.FindHarnessConfigDir(opts.Name, opts.ProjectPath, opts.TemplatePaths...)
}Same gap as run.go — provision.go:735 called harness.Resolve without ConfigDirPath, so container-script harnesses were invisible in broker mode during the provisioning step.
1 participant
Summary
harness.Resolve()usedFindHarnessConfigDirwhich only searches local disk (template, project, global dirs). Hub-managed harness configs hydrated into temp directories were invisible to the resolver.Generic{}with no command config, producing an empty shell command (sh -c "") — the agent exited immediately with code 0.ConfigDirPathtoResolveOptionsso the broker can pass the Hub-hydrated path directly.run.gonow passesopts.HarnessConfigPaththrough to the resolver.Test plan
no_auth.behavior: drop-to-shell— verify drop to bash shell with guidance messagego test ./pkg/harness/...passes